Uk national #157

peterdudfield · 2025-12-17T10:25:15Z

Pull Request

Description

Move uk-national over to this quartz-api

National routes
GSP routes
status routes - this loads from the old database for the moment. This will change in the future
system routes

Currently

forecast/all takes about ~13 seconds, which is to slow (benchmark from old app was). We currently looping over ForecastAtTimestamp
pvlive/all takes about ~10 seconds

#147

How Has This Been Tested?

CI tests
ran locally, and UAT from airflow run against the API
deployed and ran on aws,
and UAT from airflow run against the API
deploy UI to use this API - link

dont do in this PR: add end to end test for uk-national with the app #179
dont do in this PR: add seperate UAT in airflow that tests it

Checklist:

My code follows OCF's coding style guidelines
I have performed a self-review of my own code
I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works
I have checked my code and corrected any misspellings

…nal-and-gsp-locations # Conflicts: # src/quartz_api/cmd/main.py # src/quartz_api/internal/backends/dataplatform/client.py # src/quartz_api/internal/models.py

…nal-and-gsp-framework # Conflicts: # src/quartz_api/cmd/main.py

…-locations # Conflicts: # src/quartz_api/cmd/main.py # src/quartz_api/internal/service/uk_national/gsp.py # src/quartz_api/internal/service/uk_national/national.py # src/quartz_api/internal/service/uk_national/system.py

# Conflicts: # src/quartz_api/internal/service/uk_national/national.py

…/forecast

# Conflicts: # src/quartz_api/internal/service/uk_national/national.py

# Conflicts: # src/quartz_api/internal/service/uk_national/gsp.py # src/quartz_api/internal/service/uk_national/national.py # src/quartz_api/internal/service/uk_national/system.py

# Conflicts: # src/quartz_api/internal/backends/dataplatform/client.py

# Conflicts: # src/quartz_api/internal/service/uk_national/description.py

# Conflicts: # pyproject.toml # src/quartz_api/cmd/main.py # src/quartz_api/internal/backends/dataplatform/client.py # src/quartz_api/internal/backends/dummydb/client.py # src/quartz_api/internal/backends/quartzdb/client.py # src/quartz_api/internal/models/__init__.py # src/quartz_api/internal/models/db_interface.py # uv.lock

peterdudfield · 2026-01-13T18:58:46Z

src/quartz_api/internal/service/uk_national/cache.py

+        repr(sorted(params)),
+    ])
+
+    log.info(f"Cache key generated: {key}")


probably remove

# Conflicts: # src/quartz_api/internal/backends/dataplatform/client.py # src/quartz_api/internal/models/endpoint_types.py

devsjc

Not a finished review, haven't got all the way down the files yet!

devsjc · 2026-01-16T18:00:41Z

src/quartz_api/internal/backends/utils.py

+def get_window(
+    start: datetime | None = None, end:datetime | None = None,
+) -> tuple[datetime, datetime]:
    """Returns the start and end of the window for timeseries data."""
    # Window start is the beginning of the day two days ago
-    start = (dt.datetime.now(tz=dt.UTC) - dt.timedelta(days=2)).replace(
-        hour=0,
-        minute=0,
-        second=0,
-        microsecond=0,
-    )
+    if start is None:
+        start = (datetime.now(tz=UTC) - timedelta(days=2))
+        start = floor_6_hours_dt(start)
+
    # Window end is the beginning of the day two days ahead
-    end = (dt.datetime.now(tz=dt.UTC) + dt.timedelta(days=2)).replace(
-        hour=0,
-        minute=0,
-        second=0,
-        microsecond=0,
-    )
+    if end is None:
+        end = start + timedelta(days=4)
+


I'd be tempted to get rid of this altogether and just mandate that all the timeseries routes have a start and an end

yea, somewhere the logic has to sit, as people pull API with no start or end datetimes in current uk-national api, so we have to make them.

Is it possible with FastAPI to just have them in the routes but make them optional such that they default to current time +/- 2 days or whatever it's meant to be?

yep, there is probably a way to do that. So move it from the backend to the routes (and specifically the uk-national and ruvnl region routes)

I'd say so - would save having to have this file at least!

devsjc · 2026-01-16T18:01:24Z

src/quartz_api/internal/models/db_interface.py

        model_name: str | None = None,
+        start_datetime: datetime | None = None,
+        end_datetime: datetime | None = None,
+        created_utc_upper_limit: datetime | None = None,


Is created_before_datetime more in line with the other fields?

yea, im happy to change it to that

src/quartz_api/internal/models/endpoint_types.py

devsjc · 2026-01-16T18:03:05Z

src/quartz_api/internal/models/endpoint_types.py

+class Region(BaseModel):
+    """Region metadata."""
+
+    region_name: str = Field(..., json_schema_extra={"description": "The name of the region."})
+    region_metadata: dict | None = Field(
+        None,
+        json_schema_extra={"description": "Additional metadata about the region."},
+    )
+


Can this not be a subclass of LocationPropertiesBase?

Could do, does LocationPropertiesBase force a lat/lon where as a Region doesnt?

Everything has a lat/lon, even regions, as they have a centroid/associated point. So it's not a problem for region to have a lat/lon.

I'm actually tempted to leave this, its just been moved from below, and tidy this up another time.

devsjc · 2026-01-16T18:04:25Z

src/quartz_api/internal/service/uk_national/cache.py

+    func: Callable, # noqa
+    namespace: str = "",
+    *,
+    request: Request = None,
+    response: Response = None,   # noqa
+    args,  # noqa
+    kwargs,   # noqa


Do you need all these #noqa's? You have some type hinting - also you never use args or kwargs

I thought you need a #noqa per line, but i could be wrong.

I look into removing the ones I dont need, but i might be the cache function needs certain inputs

You do need it per line, but what I meant was, are they doing anything? If you remove the unused arguments does something complain?

ill check, it might be the cache function has to have them, but ill check

peterdudfield · 2026-01-19T09:51:37Z

src/quartz_api/internal/service/uk_national/cache.py

+log = logging.getLogger(__name__)
+
+
+async def key_builder(


todo add permission in cache key, for intraday users

devsjc · 2026-01-19T11:32:10Z

src/quartz_api/internal/backends/dataplatform/client.py

    ) -> list[models.PredictedPower]:
        values = await self._get_predicted_power_production_for_location(
-            location_uuid=UUID(location),
+            location_uuid=location,


Wouldn't it be safer to make the location input to the function a UUID type? Then we can enforce it at the route validation level

devsjc · 2026-01-19T11:32:26Z

src/quartz_api/internal/backends/dataplatform/client.py

            forecast_horizon_minutes=forecast_horizon_minutes,
            smooth_flag=smooth_flag,
            oauth_id=None,
+            model_name=model_name,


We use forecaster now (or try to!)

ok i will change that

devsjc · 2026-01-19T11:35:42Z

src/quartz_api/internal/backends/dataplatform/client.py

+    async def get_forecast_metadata(
+        self,
+        location_uuid: str,
+        authdata: dict[str, str],  #noqa: ARG002
+        model_name: str | None = None,
+    ) -> models.ForecastMetadata:
+        """Get forecast metadata for a site."""
+        req = dp.GetLatestForecastsRequest(
+                location_uuid=location_uuid,
+                energy_source=dp.EnergySource.SOLAR,
+            )
+        resp = await self.dp_client.get_latest_forecasts(req)
+
+        # Filter by model name if provided
+        if model_name:
+            resp.forecasts = [
+                forecast for forecast in resp.forecasts
+                if forecast.forecaster.forecaster_name == model_name
+            ]
+
+        resp.forecasts.sort(
+            key=lambda f: f.created_timestamp_utc,
+            reverse=True,
+        )
+        forecasts = resp.forecasts[0]
+
+        return models.ForecastMetadata(
+            initialization_timestamp_utc=forecasts.initialization_timestamp_utc,
+            created_timestamp_utc=forecasts.created_timestamp_utc,
+            forecaster_name=forecasts.forecaster.forecaster_name,
+            forecaster_version=forecasts.forecaster.forecaster_version,
+        )


This entire function just to return an init_timestamp of the latest forecast seems unecessary and ripe for overuse; surely everything that wants this can just get it as part of their response for their own query?

its also use in the national route where we return metadata, so i think we need all this

In my other comments I tried to describe that the metadata that is fetched by this route is already surfaced by the Data Platform's GetForecastAsTimeseries route. So all that has to be done is for that data to be surfaced in the _get_solar_power_production_for_timestamp DBClient function and it can be passed into the output of the national route without making any extra calls to the database.

devsjc · 2026-01-19T11:38:24Z

src/quartz_api/internal/backends/dataplatform/client.py

+    @override
+    async def get_forecast_for_multiple_locations(
+        self,
+        location_uuids_to_location_ids: dict[str, int],
+        authdata: dict[str, str],
+        start_datetime_utc: dt.datetime | None = None,
+        end_datetime_utc: dt.datetime | None = None,
+        model_name: str | None = None,
+    ) -> list[models.OneDatetimeManyForecastValuesMW,
+]:
+        """Get a forecast for multiple sites.
+
+        Args:
+            location_uuids_to_location_ids: A mapping from location UUIDs to location IDs.
+            authdata: Authentication data for the user.
+            start_datetime_utc: The start datetime for the prediction window. Default is None.
+            end_datetime_utc: The end datetime for the prediction window. Default is None.
+            model_name: The name of the forecasting model to use. Default is None.
+
+        Returns:
+            A list of OneDatetimeManyForecastValuesMW objects.
+        """
+        start, end = get_window(start=start_datetime_utc, end=end_datetime_utc)
+
+        # timestamps 30 mins apart from start to end
+        n_half_hours = int((((end - start).total_seconds() // 60) // 30) + 1)
+        timestamps = [start + dt.timedelta(minutes=30 * x) for x in range(n_half_hours)]
+
+        # get forecasters
+        req = dp.ListForecastersRequest(forecaster_names_filter=[model_name],
+                                      latest_versions_only=True)
+        resp = await self.dp_client.list_forecasters(req)
+        forecaster = resp.forecasters[0]
+
+        forecasts_per_timestamp = []
+        tasks = []
+        for timestamp in timestamps:
+            req = dp.GetForecastAtTimestampRequest(
+                location_uuids=list(location_uuids_to_location_ids.keys()),
+                energy_source=dp.EnergySource.SOLAR,
+                timestamp_utc=timestamp,
+                forecaster=forecaster,
+            )
+            # resp = await self.dp_client.get_forecast_at_timestamp(req)
+            task = asyncio.create_task(self.dp_client.get_forecast_at_timestamp(req))
+            tasks.append(task)
+        list_results = await asyncio.gather(*tasks, return_exceptions=True)
+        for exc in filter(lambda x: isinstance(x, Exception), list_results):
+            raise exc
+
+        for resp in list_results:
+
+            if len(resp.values) == 0:
+                continue
+
+            forecasts_one_timestamp = models.OneDatetimeManyForecastValuesMW(
+                datetime_utc=resp.timestamp_utc,
+                forecast_values={
+                    location_uuids_to_location_ids[forecast.location_uuid]: round(
+                        forecast.value_fraction * forecast.effective_capacity_watts / 10**6, 2)
+                    for forecast in resp.values
+                })
+
+            # sort by dictionary by keys
+            forecasts_one_timestamp.forecast_values =\
+                dict(sorted(forecasts_one_timestamp.forecast_values.items()))
+
+            forecasts_per_timestamp.append(forecasts_one_timestamp)
+
+        return forecasts_per_timestamp
+
+    @override
+    async def get_generation_for_multiple_locations(
+        self,
+        location_uuids_to_location_ids: dict[str, int],
+        authdata: dict[str, str],
+        start_datetime: dt.datetime | None = None,
+        end_datetime: dt.datetime | None = None,
+        observer_name: str = "ruvnl",
+    ) -> list[models.GSPYieldGroupByDatetime]:
+        """Get a forecast for multiple sites."""
+        start, end = get_window(start=start_datetime, end=end_datetime)
+
+        tasks = []
+        for location_uuid in location_uuids_to_location_ids:
+            req = dp.GetObservationsAsTimeseriesRequest(
+                location_uuid=location_uuid,
+                observer_name=observer_name,
+                energy_source=dp.EnergySource.SOLAR,
+                time_window=dp.TimeWindow(
+                    start_timestamp_utc=start,
+                    end_timestamp_utc=end,
+                ),
+            )
+            task = asyncio.create_task(self.dp_client.get_observations_as_timeseries(req))
+            tasks.append(task)
+            # observation = await self.dp_client.get_observations_as_timeseries(req)
+            # observations.append(observation)
+
+        list_results = await asyncio.gather(*tasks, return_exceptions=True)
+        for exc in filter(lambda x: isinstance(x, Exception), list_results):
+            raise exc
+
+        # Combine results into GSPYieldGroupByDatetime
+        observations_by_datetime = {}
+        for observation in list_results:
+
+            location_id = location_uuids_to_location_ids[observation.location_uuid]
+
+            for value in observation.values:
+                timestamp = value.timestamp_utc
+                if timestamp not in observations_by_datetime:
+                    # make a dictionary generation_kw_by_gsp_id
+                    observations_by_datetime[timestamp] = {}
+
+                generation_kw = int(value.effective_capacity_watts * value.value_fraction / 1000.0)
+                observations_by_datetime[timestamp][location_id] = generation_kw
+
+        # format to list of GSPYieldGroupByDatetime
+        observations_by_datetime_formated = [
+            models.GSPYieldGroupByDatetime(
+                datetime_utc=timestamp,
+                generation_kw_by_gsp_id=dict(sorted(generation_kw_by_gsp_id.items())),
+            )
+            for timestamp, generation_kw_by_gsp_id in observations_by_datetime.items()
+        ]
+        return observations_by_datetime_formated


I don't think either of these functions should exist. I'm aware they might have to for this specific PR, but it goes against the desiagn of the data platform to do these massive 2D calls.

should i put it in the route then? and then its less likely to be re-used?

Oh that's possibly sensible actually

devsjc · 2026-01-19T11:38:41Z

src/quartz_api/internal/backends/dummydb/client.py

+    @override
+    async def get_forecast_metadata() -> models.ForecastMetadata:
+        raise NotImplementedError()


This might not be needed

devsjc · 2026-01-19T11:39:16Z

src/quartz_api/internal/backends/utils.py

@@ -1,22 +1,41 @@
 """Utility functions..."""

-import datetime as dt


Why modify this? It's now different to every other file

i can change this back.

devsjc · 2026-01-19T11:44:34Z

src/quartz_api/internal/backends/dataplatform/client.py

+            resp = await self.dp_client.list_forecasters(req, metadata={"traceid": traceid})
+            forecaster = resp.forecasters[0]

        req = dp.GetForecastAsTimeseriesRequest(


The response to this returns the created_at and init_time of the forecast that produced each value in the timeseries. If we surfaced this into the models.PredictedPower dataclass, there would be no need for the get_forecast_metadata route.

Yea, thats one way to do it.

The tricky thing is, sometimes we want this metadata, and sometimes we dont. The real stripped down version should not get it, we should just get a timeseries. This is like an option extra, that users can request

But then you already have it regardless, so you might as well pass it up to the top - and then put the conditional solely on writing it back to the user at the very end of the route logic. There is no "stripping down" to be achieved by not passing it through; the data platform returns it regardless every time. In fact, all the "stripped down" version does at the moment is not do the extra (unecessary) call to the database via get_forecast_metadata (so really it's the other version that's "stripped up"!)

so does there need to be a change in data-platform for this?

No, the data platform is already returning the init time and created time alongside every forecast value when you call GetForecastAsTimeseries. All that has to be done is for the DBClient's _get_predicted_solar_power_for_location function to actually propogate these values up the stack in their returned dataclasses. The National route can then either show or not show those values based on user preference.

devsjc · 2026-01-19T11:46:38Z

src/quartz_api/internal/service/uk_national/status.py

+    forecast = await db.get_forecast_metadata(
+        location_uuid=national_location_uuid,
+        authdata={},
+        model_name=model_name,
+    )


This should call a more generic function that just gets the latests forecasts and then filter the output of that by name. I think a DBClient function like that would be more generally useful.

What do you mean by DBClient function here?

As in, and extra route on the DBClient interface that is get_latest_forecasts or something similar.

ah i see, so its basically renaming this?

Well, renaming it, and changing it's function. From what I can tell, what this function actually does in it's current state is

get the latest forecast to run for the given location (and optional forecaster name)

return the initilisation time of that forecast

So get_latest_forecast_init_time_for_location is a more accurate name, but, as I've described, you don't need a new db call for this, as that metadata is already returned when you get a forecast by the data platform. As such I think this function should be completely overhauled to just be a more generic get_latest_forecasts function.

devsjc · 2026-01-19T11:47:55Z

src/quartz_api/internal/service/uk_national/national.py

+    if include_metadata:
+        forecast_metadata: ForecastMetadata \
+            = await db.get_forecast_metadata(location_uuid=national_location_uuid,
+                                             model_name=model_name,
+                                             authdata=auth)


This whole thing could be removed as the value can be got from the get_predicted_solar_power_production_for_location call below (if the previoulsy mentioned changes are made)

peterdudfield added 30 commits December 2, 2025 16:17

add framework for UK national and GSP

0aab57c

add gsp locations route

ac833c2

lint

f35f5b9

Merge branch 'main' into uk-national-and-gsp-framework

7a60342

Merge branch 'main' into uk-national-and-gsp-locations

5128fd3

PR comments

c382360

Update TODO elexon

caba26b

remove legacy routes

8d09c92

add start to getting national forecast

d16be98

Merge commit 'b1c66bee95de4ee6e27f7a4aa27865bd039d5523' into uk-natio…

c18b8be

…nal-and-gsp-locations # Conflicts: # src/quartz_api/cmd/main.py # src/quartz_api/internal/backends/dataplatform/client.py # src/quartz_api/internal/models.py

Merge commit 'b1c66bee95de4ee6e27f7a4aa27865bd039d5523' into uk-natio…

6203f51

…nal-and-gsp-framework # Conflicts: # src/quartz_api/cmd/main.py

rename + lint

3c7e911

Merge branch 'uk-national-and-gsp-framework' into national/forecast

3110128

# Conflicts: # src/quartz_api/internal/service/uk_national/national.py

get working for model_name

f01f99c

Merge commit 'ba856c2ed063512c437dbc9cd10472bf3d1ec146' into national…

618d0c7

…/forecast

Merge branch 'main' into national/forecast

b9b3fb0

# Conflicts: # src/quartz_api/internal/service/uk_national/national.py

Merge branch 'main' into uk-national-and-gsp-locations

702b73b

# Conflicts: # src/quartz_api/internal/service/uk_national/gsp.py # src/quartz_api/internal/service/uk_national/national.py # src/quartz_api/internal/service/uk_national/system.py

lint

e3e068f

lint

0996fbf

national/pvlive

7ac4a80

lint

b038e79

Merge branch 'national/pvlive' into uk-national-and-gsp-locations

cce60d6

# Conflicts: # src/quartz_api/internal/backends/dataplatform/client.py

tidy up

332f540

add options for get_window

2a8cb85

Merge branch 'start-end' into national/forecast

74ee1d6

add start and end datetime

91445bf

add start and end filters

539c4c8

lint

9297666

Merge branch 'start-end' into national/pvlive

0469b5b

peterdudfield added 20 commits January 6, 2026 17:26

lint

19ca23a

Merge branch 'main' into uk-national

b5a5b0b

# Conflicts: # src/quartz_api/internal/service/uk_national/description.py

Merge commit 'd952b899c7d4c01cbefa6be20c5959587bb21373' into uk-national

ac336a2

use dataplatform 0.18.0

f08e3dd

use created_utc_limit

fdecea6

update model names from - to _

5722ffc

order by gsp_id

e143a3f

system/GB/gsp/

a176508

solar/GB/status no solar/GB/status/

315ffc6

remove empty forecasts in gsp/forecast/all

a1ec834

add cache

559f3fa

change window to -2day, floored 2 6 hours

d196d19

fix for forecast horizon

fc0d218

move gsp names to upper

bca002c

forecast/all in mw

7e3dbba

add longer cache for forecast/all and pvlive/all

0c73ec5

remove old params in cache key builder

7cb1ece

add logging to cache

ac276fb

small fixes

c1b6d90

peterdudfield commented Jan 13, 2026

View reviewed changes

peterdudfield added 3 commits January 16, 2026 12:05

Merge commit 'ad46de99b2ad917d0ddb0488f5df3f33a7115d34' into uk-national

baeabb3

# Conflicts: # src/quartz_api/internal/backends/dataplatform/client.py # src/quartz_api/internal/models/endpoint_types.py

copy over fixes

a89692c

cover gsp_ids='non-int-string' case

6234606

devsjc reviewed Jan 16, 2026

View reviewed changes

peterdudfield commented Jan 19, 2026

View reviewed changes

devsjc reviewed Jan 19, 2026

View reviewed changes

peterdudfield and others added 3 commits January 19, 2026 12:48

Merge commit 'b057d444a20e4e47e236d141cef1c59afb1e32f9' into uk-national

b3cf6db

PR changes

d327ee8

Merge branch 'main' into uk-national

ba06acb

		@@ -1,22 +1,41 @@
		"""Utility functions..."""

		import datetime as dt

Uh oh!

Uk national #157

Are you sure you want to change the base?

Uk national #157

Conversation

peterdudfield commented Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request

Description

How Has This Been Tested?

Checklist:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devsjc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devsjc Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

peterdudfield commented Dec 17, 2025 •

edited

Loading

devsjc Jan 19, 2026 •

edited

Loading